Skip to content

feat(payjoin): settings, DB v14 migration, and live status - #2477

Merged
i5hi merged 7 commits into
payjoin-upgradefrom
pj/00-settings-db
Jul 21, 2026
Merged

feat(payjoin): settings, DB v14 migration, and live status#2477
i5hi merged 7 commits into
payjoin-upgradefrom
pj/00-settings-db

Conversation

@ethicnology

@ethicnology ethicnology commented Jul 20, 2026

Copy link
Copy Markdown
Member

Adds the Drift v14 migration for payjoin, a reactive enabled/disabled setting (payjoinEnabledChangeStream), and a proper ServiceStatus.disabled value so the Status Services screen reports "Disabled" instead of "Offline"/"Unknown" when the user has turned payjoin off.

Relates to #782 (Allow user to view payjoin settings), #1584 (Default payjoin settings), #947 (Better Payjoin Status).

Merge order: 2nd, right after #2443. Base of PR2 (pj/01-core-engine). Must merge before PR2-PR5.
#2443 → PR1 (this) → PR2 → {PR3, PR4, PR5}

@ethicnology
ethicnology requested a review from i5hi July 20, 2026 20:49
@ethicnology ethicnology self-assigned this Jul 20, 2026
Adds is_aborted to payjoin_receivers/payjoin_senders and
is_payjoin_enabled / payjoin_min_amount_sat / payjoin_expire_after_sec
to settings, migrated in schema_13_to_14 with backfilled defaults
(disabled, 10000 sat, 24h). PayjoinConstants gains the bounds and the
documented 24h-expiry rationale.

The bulk of the diff is the generated v14 schema snapshot; review the
migration step, the table definitions, the seeds and the constants.
Threads the three new payjoin settings from the settings entity through
the repository and datasource, fed by the DB columns added in the v14
migration.
Adds PayjoinStatus.aborted (a fallback broadcast: the payment landed as
a plain transaction) and the isAborted model field, mapped to/from the
new DB column and round-tripped in fromReceiverTable/fromSenderTable
(fixing a pre-existing bug where isExpired/isCompleted were never mapped
back, silently resetting a re-fetched session's status). The CSV export
gains the matching aborted case so its status switch stays exhaustive.

Entity getters logRef and canManuallyBroadcastOriginal are introduced
here as the shared vocabulary the core engine (next) builds on.
Adds the payjoin settings screen (auto-saving toggle, min-amount and
expiry with bounds-validating use-cases) reachable from bitcoin
settings, wired through the settings cubit and locator. Includes all new
payjoin l10n keys (settings + the send/receive/transactions keys the
later feature PRs consume) so every stacked branch compiles standalone.
Adds a payjoinEnabledChangeStream to SettingsRepository (mirrors the
existing currencyChangeStream pattern), emitted from setPayjoinEnabled,
plus a WatchPayjoinEnabledChangesUsecase. Lets a live listener (the
receive flow) react to the global setting changing without needing to
leave and re-enter its screen.
Adds ServiceStatus.disabled (intentionally turned off, distinct from
unknown/not-checked) and returns it for the payjoin service the moment
the setting is off, instead of probing the OHTTP relay and painting the
whole status page red for a feature the user isn't relying on. The
status page shows it muted-grey with a "Disabled" label, and it no
longer counts against allServicesOnline.
@ethicnology

Copy link
Copy Markdown
Member Author

Heads-up: force-pushed to fold the dismissed_announcements table into the existing v14 migration (the v14 commit changed SHA).

Why the new table lives here: the upcoming home-announcements feature (#<pj/07 PR number>) needs a persisted "dismissed announcements" table. Rather than ship a separate v15 migration stacked on top, we add the table to this PR's v14 migration — one schema bump instead of two, one Drift snapshot instead of two (~8k fewer generated lines across the two PRs). The table is created empty, so it's an additive, backfill-free change with no impact on the payjoin work in this PR.

#2484

@ethicnology

Copy link
Copy Markdown
Member Author

@claude audit this PR please

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown

Claude finished @ethicnology's task in 6m 57s —— View job


Audit: feat(payjoin): settings, DB v14 migration, and live status

  • Gather context (PR diff vs origin/payjoin-upgrade, 6 commits, 68 files)
  • Review DB v14 migration & schema changes
  • Review settings plumbing (datasource → repository → usecases → cubit → UI)
  • Review payjoin model/entity changes (aborted status)
  • Review status service changes (ServiceStatus.disabled)
  • Review tests & migration tests
  • Post audit findings

Verdict: no blocking issues found. The migration is additive and correct, the settings plumbing is consistent end-to-end, and the aborted/disabled semantics are coherent. A few forward-compat and polish items below.

Should fix (or explicitly confirm PR2 covers it)

1. onlyOngoing DB queries don't exclude aborted sessionslib/core/payjoin/data/datasources/local_payjoin_datasource.dart:123-127 and :146-150 filter isExpired(false) + isCompleted(false) only. This PR redefines "ongoing" at the entity level to exclude aborted (payjoin.dart:84: !isCompleted && !isAborted && !isExpired), but the DB-level resume filter wasn't updated to match. Inert in this PR (nothing writes isAborted = true yet), but the moment PR2 starts setting the flag, aborted sessions would still be picked up by the resume/watcher path. Adding .filter((f) => f.isAborted(false)) here is harmless now and correct later. Fix this →

2. Deleted de/fr translations for still-live keyslocalization/app_de.arb and app_fr.arb drop their translations of receivePayjoinActivated (de also drops receivePayjoinNoUtxos), while the keys remain in the template (app_en.arb:14883-14890, relocated, values unchanged) and are still used by receive_qr_screen.dart:644,671. German/French users regress to English for previously-translated strings. If the deletions were deliberate (stale wording), fine — otherwise restore them.

Consider

3. Debounced field edits are silently dropped on back-navigationpayjoin_settings_screen.dart: the 500 ms debounce timers are cancelled in dispose(), so typing a valid min-amount/expiry and leaving the screen within the window loses the change with no feedback. Flushing a pending valid value in dispose() (persist immediately instead of cancel) would close that gap.

4. Switch toggle is fire-and-forget — the BBSwitch.onChanged calls togglePayjoinEnabled without the error handling the two text fields get via _persist (payjoin_settings_screen.dart:~215). On a DB failure the future rejects unhandled; routing it through _persist too would keep failure handling symmetric. Related edge: SettingsCubit.togglePayjoinEnabled no-ops the emit when storedSettings is still null while the DB write proceeds — harmless today since the screen sits behind an initialized cubit, but worth knowing.

5. Forward-declared, currently-unused APIPayjoin.logRef / logRefForId / canManuallyBroadcastOriginal and WatchPayjoinEnabledChangesUsecase have no callers in this PR, and some doc comments reference symbols that don't exist on this branch yet (_broadcastOriginalTransaction, _onOriginalTransactionSeen). Understood as PR2 staging — but canManuallyBroadcastOriginal claims to exactly mirror PayjoinRepositoryImpl.tryBroadcastOriginalTransaction's guard, and since that guard lives in code this PR doesn't touch, that equivalence needs re-verification in the PR2 review when both sides are visible.

6. Settings fetch failure reports payjoin as "offline"check_all_service_status_usecase.dart:153: the _settingsRepository.fetch() sits inside the same try/catch as the relay probe, so a settings-read failure paints payjoin red without the relay ever being probed. Negligible in practice; noting for completeness.

Verified good

  • Migration (schema_13_to_14.dart): purely additive, all five new columns carry DB defaults so backfill is automatic; duplicate-column guards match the established schema_12_to_13.dart pattern; dismissed_announcements is created empty. The migration test covers all three settings backfills, both is_aborted backfills, and the new table's writability. The keys relocated in app_en.arb are unchanged in value (I initially flagged them as removed — they were moved, not deleted).
  • fromReceiverTable/fromSenderTable now mapping isExpired/isCompleted is a real pre-existing bug fix (re-fetched sessions previously always reported as unresolved), and isAborted correctly rides along.
  • Status ordering completed > aborted > expired in payjoin_model.dart is right for the documented double-flag race, and PayjoinStatus.aborted is handled exhaustively everywhere the enum is switched (CSV export, details table via l10n keys, status derivation).
  • ServiceStatus.disabled: allServicesOnline treats disabled as non-fault, the status page renders it muted, and statusCheckDisabled exists in the template.
  • Bounds enforcement lives in the domain usecases (not just the UI), with boundary-inclusive tests; the dedicated payjoinEnabled boolean (vs. a sentinel on min-amount) has a regression test.
  • Datasource/usecase patterns (single-row id: 1 update, data-repo imports in usecases) match the existing codebase conventions exactly.

Limitations

No Flutter toolchain is available in this runner, so I could not run flutter analyze, the test suite, or gen-l10n; the audit is static. The ~11k generated lines (schema_v14.dart, sqlite_database.steps.dart, drift_schema_v14.json) were spot-checked for the new columns, not line-audited. No CI runs were reported for this PR at audit time — worth confirming CI is green before merge.

@i5hi
i5hi merged commit 8582e06 into payjoin-upgrade Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

payjoin improments Default payjoin settings Allow user to view payjoin settings

2 participants